-
Notifications
You must be signed in to change notification settings - Fork 34
[INSTA-69727] Fysom improvement for multi-threaded operations #832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The code is tested for gunicorn in amd64, arm64 and with autotrace-webhook. |
e57efa4 to
8970564
Compare
GSVarsha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested changes.
8970564 to
123f69b
Compare
123f69b to
55842f5
Compare
GSVarsha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few suggestions
e46e111 to
7e6ce54
Compare
7e6ce54 to
390837b
Compare
src/instana/agent/host.py
Outdated
| Forks happen. Here we handle them. | ||
| """ | ||
| # Update boot PID to current PID to prevent duplicate fork detection | ||
| self._boot_pid = os.getpid() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this variable is not used anywhere. Am I wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't change anything regarding _boot_pid variable but it's used in can_send method to perform fork detection.
| if lock_acquired: | ||
| self.collector.background_report_lock.release() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may not be following correctly, but why are you releasing the lock in a function that mainly prints a diagnostic of the agent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, we were using the locked() method to check if the background_report_check is locked or not but as we moved to RLock, this new object doesn't have this method. We acquire the thread without blocking and release it immediately.
…nstrumentation Signed-off-by: Cagri Yonca <cagri@ibm.com>
390837b to
da98753
Compare
|



fysom.FysomError: event announce inappropriate in current state announcederror. After adding a thread-safe lock mechanism, it works and propagates spans as expected.